Conversation
jayb
requested review from
Mgetz10,
Viseghoh,
adamdoe and
khartco
as code owners
There was a problem hiding this comment.
Pull request overview
Adds a new configuration option for nested dropdown filters to display only the selected subgroup in the closed input, along with several nested dropdown UX/styling adjustments and accompanying tests/stories.
Changes:
- Introduces
displaySubgroupingOnlyconfig flag and editor checkboxes to control nested dropdown closed-display text. - Updates NestedDropdown UX/styling (caret behavior, focus outline placement, input focus behavior).
- Adds/updates unit tests and Storybook stories covering the new option and updated behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dashboard/src/types/SharedFilter.ts | Adds displaySubgroupingOnly to dashboard shared filter type. |
| packages/dashboard/src/components/DashboardFilters/DashboardFiltersEditor/components/NestedDropDownDashboard.tsx | Updates subgroup-related label/comment text. |
| packages/dashboard/src/components/DashboardFilters/DashboardFiltersEditor/components/FilterEditor.tsx | Adds “Display subgrouping only” checkbox in dashboard filter editor. |
| packages/dashboard/src/components/DashboardFilters/DashboardFiltersEditor/components/FilterEditor.test.tsx | Tests dashboard FilterEditor rendering/toggling for the new checkbox. |
| packages/dashboard/src/components/DashboardFilters/DashboardFilters.tsx | Passes displaySubgroupingOnly through to NestedDropdown. |
| packages/dashboard/src/components/DashboardFilters/DashboardFilters.test.tsx | Tests closed-display text and selection behavior with/without subgroup-only display. |
| packages/dashboard/src/_stories/Dashboard.stories.tsx | Adds a dashboard story variant for subgroup-only display + updates assertions. |
| packages/core/types/VizFilter.ts | Adds displaySubgroupingOnly to core filter type used by Filters/Viz editors. |
| packages/core/components/NestedDropdown/NestedDropdown.tsx | Implements subgroup-only closed display and focus/interaction changes. |
| packages/core/components/NestedDropdown/nesteddropdown.styles.css | Updates nested dropdown visuals (outline placement, caret sizing/orientation, typography). |
| packages/core/components/NestedDropdown/tests/NestedDropdown.test.tsx | Adds unit coverage for subgroup-only display + escape/search behaviors. |
| packages/core/components/Filters/Filters.tsx | Passes displaySubgroupingOnly to NestedDropdown in core Filters. |
| packages/core/components/EditorPanel/VizFilterEditor/NestedDropdownEditor.tsx | Adds subgroup-only checkbox in viz nested dropdown editor. |
| packages/core/components/EditorPanel/VizFilterEditor/NestedDropdownEditor.test.tsx | Tests viz editor checkbox placement and update callback. |
| packages/core/components/ComboBox/combobox.styles.css | Tweaks combobox option sizing to better align with updated dropdown styling. |
| packages/core/components/_stories/NestedDropdown.stories.tsx | Updates stories to explicitly demonstrate default vs subgroup-only display. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/core/components/NestedDropdown/nesteddropdown.styles.css
Outdated
Show resolved
Hide resolved
...dashboard/src/components/DashboardFilters/DashboardFiltersEditor/components/FilterEditor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleOnBlur = (e: FocusEvent): void => { | ||
| const relatedTarget = e.relatedTarget as Node | null | ||
|
|
||
| if (!relatedTarget || !nestedDropdownRef.current?.contains(relatedTarget)) { | ||
| resetDropdownInteraction() |
There was a problem hiding this comment.
handleOnBlur is typed as a native FocusEvent, but this handler is also passed to React onBlur props (e.g., the group <li>), which receive React.FocusEvent. This type mismatch makes the intent unclear and can hide real issues. Consider standardizing on a React onBlur handler attached to the wrapper (React onBlur bubbles) and using React.FocusEvent typing, instead of mixing native focus events.
| {filter.filterStyle === FILTER_STYLE.nestedDropdown && ( | ||
| <label> | ||
| <input | ||
| type='checkbox' | ||
| checked={!!filter.displaySubgroupingOnly} | ||
| aria-label='Display subgrouping only' | ||
| onChange={e => updateFilterProp('displaySubgroupingOnly', e.target.checked)} | ||
| /> | ||
| <span> Display subgrouping only</span> | ||
| </label> | ||
| )} |
There was a problem hiding this comment.
This checkbox is rendered inside the filter.filterStyle !== FILTER_STYLE.nestedDropdown ? ... : ... nested-dropdown branch, so the extra filter.filterStyle === FILTER_STYLE.nestedDropdown condition is always true here and can be removed to simplify the JSX.
| {filter.filterStyle === FILTER_STYLE.nestedDropdown && ( | |
| <label> | |
| <input | |
| type='checkbox' | |
| checked={!!filter.displaySubgroupingOnly} | |
| aria-label='Display subgrouping only' | |
| onChange={e => updateFilterProp('displaySubgroupingOnly', e.target.checked)} | |
| /> | |
| <span> Display subgrouping only</span> | |
| </label> | |
| )} | |
| <label> | |
| <input | |
| type='checkbox' | |
| checked={!!filter.displaySubgroupingOnly} | |
| aria-label='Display subgrouping only' | |
| onChange={e => updateFilterProp('displaySubgroupingOnly', e.target.checked)} | |
| /> | |
| <span> Display subgrouping only</span> | |
| </label> |
Summary
Adds option to display only subgrouping in nested dropdowns. Also makes a few usability improvements for nested dropdowns:
Testing Steps
This config contains a nested dropdown. Experiment with the new "Display subgrouping only" option in the filter editor.